TST: Ensure 100% type-completeness (according to Pyright), check it in CI #1628
Conversation
Dr-Irv
left a comment
Dr-Irv
left a comment
There was a problem hiding this comment.
I'm OK with this. But I think it would be useful to have some documentation in docs/tests.md that explains how to run this locally (assuming it can). And, if so, can it be hooked into the poe structure for consistency?
| subprocess.run(cmd, check=True) | ||
|
|
||
|
|
||
| def type_completeness() -> None: |
There was a problem hiding this comment.
How long does that step take? It could be worth adding to the test_all step that one would run locally when developing.
There was a problem hiding this comment.
looks like it's 2 minutes 26 seconds 😩
I know that pyrefly also wants to add this functionality, and maybe we can get ty to add it too, so hopefully there'll be a faster way to check this is in the future
do you still want it in test_all or is this too slow?
There was a problem hiding this comment.
2 min is a bit long i would say, I think you would agree too, the mypy step is quite time consuming too so maybe that would add too much friction. Otherwise we could add it in the same style as the nightly steps, so only when we merge, open to options.
There was a problem hiding this comment.
2 min is a bit long i would say, I think you would agree too, the mypy step is quite time consuming too so maybe that would add too much friction. Otherwise we could add it in the same style as the nightly steps, so only when we merge, open to options.
Doesn't need to be in test_all, but one should be able to run it locally. Not sure if that is already in there.
There was a problem hiding this comment.
Yes it would be already there, I think with the time it takes it is better to just run it on GitHub CI and not in test_all to reduce friction (it is fine if CI takes a bit of time)>
There was a problem hiding this comment.
yup, poetry run poe type_completeness runs
|
/pyright_strict |
|
Side comment: the PR prefix |
MarcoGorelli
changed the title
MarcoGorelli
changed the title
4 participants
I'm open to comments here, I'd just like to suggest using Pyright to check type compelteness in CI. NumPy do something similar but they have their stubs as part of their package so for them it's a lot simpler: https://github.com/numpy/numpy/blob/89776e23e188438637b636651ff3ce0df4bd2097/tools/pyright_completeness.py
This is different from checking type correctness. The existing jobs already cover that. This only checks that publicly exported symbols in pandas have known types
I had to rework a handful of imports due to microsoft/pyright#11196, which I haven't received a response on (yet?)